Fix preview watch target file path #339
Conversation
Resolve "EMFILE: too many open files" error caused by watching unnecessary files. - Change preview command to use fileSystemRepo.getRootPath() instead of config.getItemsRootDir() - Make getRootPath() public in FileSystemRepo for external access(Please visit if this is the right approach) - Add chokidar ignore pattern to exclude .remote directory from file watching
Summary of ChangesHello @fussy113, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request addresses a critical 'too many open files' error that users encountered when running the preview command. The solution involves a targeted optimization of the file watching mechanism. By refactoring the watch path to use a more specific root from the file system repository and explicitly excluding irrelevant directories like Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request effectively addresses the 'EMFILE: too many open files' error by refining the file watching mechanism. The changes to narrow the watch path to the public directory and explicitly ignore the .remote directory are well-targeted. Making getRootPath() public is a reasonable approach to expose the necessary path information from FileSystemRepo, and the addition of a unit test for getRootPath() is a great inclusion. I have one suggestion to improve the platform-independence of the new test case.
| it("returns the root path", () => { | ||
| const dataRootDir = "./tmp"; | ||
| const instance = new FileSystemRepo({ dataRootDir }); | ||
| expect(instance.getRootPath()).toBe(`tmp/public`); |
There was a problem hiding this comment.
Using a hardcoded path separator / can cause this test to fail on different operating systems, such as Windows. It's best practice to use path.join to construct paths for cross-platform compatibility.
You'll need to add import path from 'node:path'; at the top of the file to use this suggestion.
| expect(instance.getRootPath()).toBe(`tmp/public`); | |
| expect(instance.getRootPath()).toBe(path.join("tmp", "public")); |
There was a problem hiding this comment.
Pull Request Overview
This PR fixes an "EMFILE: too many open files" error by narrowing the file watching scope from the entire items root directory to just the public subdirectory, and explicitly excluding the .remote directory from being watched.
Key Changes:
- Changed the preview command to watch only the
public/subdirectory instead of the entire items root directory - Made
getRootPath()public inFileSystemRepoto enable external access - Added chokidar ignore pattern to exclude
.remotedirectory from file watching
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| src/server/app.ts | Added ignore pattern to exclude .remote directory from file watching |
| src/lib/file-system-repo.ts | Changed getRootPath() from private to public visibility |
| src/lib/file-system-repo.test.ts | Added test case for newly public getRootPath() method |
| src/commands/preview.ts | Switched watch path from config.getItemsRootDir() to fileSystemRepo.getRootPath() to narrow scope |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
1 participant
What
Resolve "EMFILE: too many open files" error caused by watching unnecessary files.
How
Why
Solve this discussion
Refs